Skip to content

[IMPL] - added support for sass and scss stylesheet languages#4292

Merged
adhami3310 merged 29 commits intoreflex-dev:mainfrom
KronosDev-Pro:add-sass-scss-stylesheet-support
Mar 18, 2025
Merged

[IMPL] - added support for sass and scss stylesheet languages#4292
adhami3310 merged 29 commits intoreflex-dev:mainfrom
KronosDev-Pro:add-sass-scss-stylesheet-support

Conversation

@KronosDev-Pro
Copy link
Contributor

  • better checking of stylesheets to be compiled
  • added support for sass and scss stylesheet languages
  • the stylesheets files are now copied to ".web/styles/" at compile time
  • relock poetry file for libsass deps
  • stylesheet compiler unit tests also check the contents of the file

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

 - better checking of stylesheets to be compiled
 - added support for sass and scss stylesheet languages
 - the stylesheets files are now copied to ".web/styles/" at compile time
 - relock poetry file for libsass deps
 - stylesheet compiler unit tests also check the contents of the file
Copy link
Member

@adhami3310 adhami3310 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks much better now, a few nitpicks and we should be good to go !

@KronosDev-Pro
Copy link
Contributor Author

Sorry, that's why I can't see the error...

Unit tests, cp312
Unit tests, cp310

@KronosDev-Pro
Copy link
Contributor Author

KronosDev-Pro commented Nov 11, 2024

Units tests

python package check
cp39
+libsass
-libsass
cp310
+libsass
-libsass
cp311
+libsass
-libsass
cp312
+libsass
-libsass

@KronosDev-Pro
Copy link
Contributor Author

well, for the integration-node-latest / check_latest_node (3.12, 1, node), I don't know why he raises some errors

assert compiler.compile_root_stylesheet(stylesheets) == (
str(Path(".web") / "styles" / "styles.css"),
"@import url('../public/styles.css'); \n",
"@import url('./styles.css'); \n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure this won't break anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know where this could cause a problem, since all the supported stylesheet files in the app/assets directories and subdirectories are copied or compiled and saved in the .web/styles directory and no longer in the .web/public directories.

and also, I checked the second batch of the tests/integration group and the tests/integration/test_tailwind.py (which is the only one to have tests impacted by the changes) passed the tests without any errors, which is why I don't understand the error in the first batch of the tests/integration group.

 - better checking of stylesheets to be compiled
 - added support for sass and scss stylesheet languages
 - the stylesheets files are now copied to ".web/styles/" at compile time
 - relock poetry file for libsass deps
 - stylesheet compiler unit tests also check the contents of the file
Copy link
Member

@adhami3310 adhami3310 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me, i'm tagging @masenf just to confirm the change from .web/public to .web/styles is a fine one.

@adhami3310 adhami3310 requested a review from masenf January 10, 2025 22:21
@adhami3310 adhami3310 closed this Jan 10, 2025
@adhami3310 adhami3310 reopened this Jan 10, 2025
@adhami3310 adhami3310 merged commit 87ed159 into reflex-dev:main Mar 18, 2025
44 of 45 checks passed
bhatia2akshit pushed a commit to bhatia2akshit/reflex that referenced this pull request Mar 19, 2025
…-dev#4292)

* [IMPL] - added support for sass and scss stylesheet languages

 - better checking of stylesheets to be compiled
 - added support for sass and scss stylesheet languages
 - the stylesheets files are now copied to ".web/styles/" at compile time
 - relock poetry file for libsass deps
 - stylesheet compiler unit tests also check the contents of the file

* fix f-string bug

* make "libsass" an optional dependency

* remove libsass of deps list

* revert peotry relock

* fix test caused by optional "libsass" deps

* improving `_compile_root_stylesheet` function and add folder stylesheets unit test

* fix the copy files in assets to public folder

* remove useless f-string

* little general improvement

* fix f-string

* remove useless path search

* remove unused var & import

* [IMPL] - added support for sass and scss stylesheet languages

 - better checking of stylesheets to be compiled
 - added support for sass and scss stylesheet languages
 - the stylesheets files are now copied to ".web/styles/" at compile time
 - relock poetry file for libsass deps
 - stylesheet compiler unit tests also check the contents of the file

* fix f-string bug

* make "libsass" an optional dependency

* remove libsass of deps list

* fix test caused by optional "libsass" deps

* improving `_compile_root_stylesheet` function and add folder stylesheets unit test

* fix the copy files in assets to public folder

* remove useless f-string

* little general improvement

* fix f-string

* remove useless path search

* remove unused var & import

---------

Co-authored-by: Khaleel Al-Adhami <khaleel.aladhami@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants